-
Notifications
You must be signed in to change notification settings - Fork 36
Sprint2__Subscriber Features 1 of 2 (Client dependent) #43
base: master
Are you sure you want to change the base?
Sprint2__Subscriber Features 1 of 2 (Client dependent) #43
Conversation
State Change on validity (days), Added Grace Period, Send SMS to subscriber on validity expiry or if about to.
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks! If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact cla@fb.com if you have any questions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome! Very neat. Two changes:
- We need to route SMS notifications locally than out the interconnect. We should perform an RPC to the client federer incoming SMS endpoint.
- A subscriber can have more than one number associated. We should vacuum them all
number__valid_through__lte=today) | ||
today = today.date() | ||
for subscriber in subscribers: | ||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should have a loop on each of the subscriber's number. there can be more than the one at index 0
""" | ||
today = django.utils.timezone.datetime.now().date() | ||
for subscriber in Subscriber.objects.iterator(): | ||
# Do nothing if subscriber vacuuming is disabled for the network. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all numbers :)
body = 'Your validity is about to get expired on %s , Please ' \ | ||
'recharge to continue the service. Please ignore if ' \ | ||
'already done! ' % (subscriber_validity,) | ||
sms_notification(body=body, to=number) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This SMS will go through SMPP SMSC/Nexmo rather than into the internal network. We should add an endpoint on federer that will get triggered via RPC. Can you change this to an RPC POST to the client? We can implement the actual client handler later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure will try to do that way.
) == today or today == ( | ||
subscriber_validity - datetime.timedelta( | ||
days=1)) or today == subscriber_validity): | ||
body = 'Your validity is about to get expired on %s , Please ' \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs be translated so this will need to live on the client. you can construct a key/short phrase that you send that the client translates with local i18n
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will update.
Thanks for reviewing and appreciation of the code. |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Please review Subscriber features which include:
--> Subscriber's State change (first expire/expired/recycle) with respect to subscriber's valid, threshold and grace days.
--> Send SMS to subscriber regarding validity expiry.